Skip to content

Improve device-init grouped linear module with single grouped weight support - #3224

Open
zhongbozhu wants to merge 23 commits into
NVIDIA:mainfrom
zhongbozhu:improve_device_grouped_linear
Open

Improve device-init grouped linear module with single grouped weight support #3224
zhongbozhu wants to merge 23 commits into
NVIDIA:mainfrom
zhongbozhu:improve_device_grouped_linear

Conversation

@zhongbozhu

@zhongbozhu zhongbozhu commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator

Description

Need this Mcore PR to make it work: NVIDIA/Megatron-LM#6000

Fixes numerical issues when using single weight for TE module grouped linear. Limit the single weight feature to the grouped tensor API instead of the legacy path.

TODO: test E2E convergence, unit test directly from Mcore.

Note: needs to pay extra attention to whether bias grad and weight grad are generated properly.

Type of change

  • Documentation change (change only to the documentation, either a fix or a new content)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Infra/Build change
  • Code refactoring

Changes

Please list the changes introduced in this PR:

  • Change A
  • Change B

Checklist:

  • I have read and followed the contributing guidelines
  • The functionality is complete
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@github-actions github-actions Bot added the community-contribution PRs from external contributor outside the core maintainers, representing community-driven work. label Jul 20, 2026
@zhongbozhu
zhongbozhu force-pushed the improve_device_grouped_linear branch from aa3b9d1 to 47ba66a Compare July 20, 2026 22:36
Comment thread transformer_engine/pytorch/module/grouped_linear.py Outdated
@zhongbozhu
zhongbozhu force-pushed the improve_device_grouped_linear branch from ff7eee2 to a43f70f Compare July 20, 2026 22:53
Comment thread transformer_engine/pytorch/module/grouped_linear.py Outdated
is_grad_enabled = torch.is_grad_enabled()
num_gemms = self.num_gemms

if FP8GlobalStateManager.fp8_graph_capturing():

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: this code block was deleted because it was duplicated

@zhongbozhu
zhongbozhu marked this pull request as ready for review July 23, 2026 10:52
@zhongbozhu

Copy link
Copy Markdown
Collaborator Author

/te-ci pytorch

@greptile-apps

greptile-apps Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR improves the device-init grouped linear module by adding single grouped weight/bias support for the native grouped-tensor GEMM path, fixing numerical issues, and refactoring the capability-gate logic into two exported helper functions (is_module_grouped_tensor_path_supported / is_op_fuser_grouped_tensor_path_supported). It also fixes make_grouped_weights to no longer skip Float8CurrentScaling parameters, adds an output= reuse argument to tex.group_quantize for CUDA-graph-safe weight caching, and introduces a use_grouped_tensor constructor flag to replace the deprecated NVTE_GROUPED_LINEAR_USE_FUSED_GROUPED_GEMM environment variable.

  • module/grouped_linear.py: Single grouped weight/bias is limited to the grouped-tensor path; the legacy split-quantize path only accepts discrete parameters. Weight-workspace caching, delayed wgrad, fused wgrad accumulation, and bias-gradient flow are all extended for grouped parameter layouts.
  • ops/basic/grouped_linear.py: Removes the inline _is_graph_safe_path_supported method and replaces it with the exported is_op_fuser_grouped_tensor_path_supported. Single grouped parameters now raise immediately when the native path is unavailable instead of falling through to an incompatible split-quantize flow.
  • C++/CUDA: group_quantize gains output= reuse support; launch_grouped_bias_add short-circuits on zero total rows; MXFP8 scale layout for uniform grouped tensors now correctly derives per-member scale bases.

Confidence Score: 4/5

PR is safe to merge; all issues found are P2 quality/maintenance concerns with no blocking defects.

The changes are well-tested with new targeted test cases covering the new single grouped weight/bias paths, CUDA graph safety, and edge cases. The key bug fixes (out_features dim, MXFP8 scale base, zero-row short-circuit, meta-device crash) are correct. The capability-gate duplication and cuBLASLt version floor change are minor P2 concerns that do not block correctness.

Files Needing Attention: module/grouped_linear.py and ops/basic/grouped_linear.py — the two is_*_grouped_tensor_path_supported predicates should ideally share logic to avoid silent divergence.

Important Files Changed

Filename Overview
transformer_engine/pytorch/module/grouped_linear.py Core of this PR: adds single grouped weight/bias support, the use_grouped_tensor flag, and the exported is_module_grouped_tensor_path_supported predicate. Duplicated capability-gate logic vs ops module; cuBLASLt version floor raised from 13.4 to 13.6 for FP8 block scaling on Hopper.
transformer_engine/pytorch/ops/basic/grouped_linear.py Refactors _is_graph_safe_path_supported into exported is_op_fuser_grouped_tensor_path_supported. New _get_packed_bias_tensor avoids split+stack for single grouped bias. Meta-device guard added to _apply_delay_wgrad_param_hooks.
transformer_engine/pytorch/csrc/extensions/cast.cpp Adds output= reuse path to group_quantize for CUDA-graph-safe weight caching. Validates shape, quantizer identity, and num_tensors consistency before reusing. Quantizer identity check is object-identity, not value equality.
transformer_engine/common/cast/mxfp8/group_quantize_mxfp8.cuh Fixes MXFP8 scale layout for SAME_BOTH_DIMS uniform grouped tensors: per-member scale bases now correctly computed as tensor_id * (first_logical_dim / num_tensors) * cols.
transformer_engine/common/gemm/cublaslt_grouped_gemm.cu Replaces null-pointer heuristics with explicit a_is_discrete/c_is_discrete/d_is_discrete flags. Zero-row short-circuit in launch_grouped_bias_add now precedes has_data() null-pointer checks.
tests/pytorch/test_grouped_linear.py Adds end-to-end tests for single grouped weight/bias across BF16, FP8, MXFP8; tests for zero-work bias null-pointer safety and host-m_splits rejection. Skip guards consolidated into is_module_grouped_tensor_path_supported.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[GroupedLinear.forward] --> B{single_grouped_weight\nor single_grouped_bias?}
    B -- Yes --> C{use_grouped_tensor?}
    C -- No --> D[RuntimeError: requires use_grouped_tensor=True]
    C -- Yes --> E{is_module_grouped_tensor_path_supported?}
    E -- No --> F[RuntimeError: native path unavailable]
    E -- Yes --> G{m_splits on CUDA?}
    G -- No --> H[ValueError: requires CUDA m_splits]
    G -- Yes --> I[_forward_grouped_tensor]
    B -- No --> J{use_grouped_tensor=True AND supported?}
    J -- Yes --> I
    J -- No --> K[Legacy split-quantize path]
    I --> L{single_grouped_weight?}
    L -- Yes --> M[_prepare_weights_for_grouped_tensor_gemm\nGroupedTensorStorage weight]
    L -- No --> N[_prepare_weights_for_grouped_tensor_gemm\nDiscrete weights list]
    M --> O{workspace cached?}
    O -- No --> P[tex.group_quantize creates workspace]
    O -- skip_fp8_weight_update --> Q[tex.group_quantize with noop_flag]
    O -- first_microbatch --> R[tex.group_quantize updates workspace]
    O -- not first_microbatch --> S[Reuse cached workspace]
    P --> T[grouped GEMM forward]
    Q --> T
    R --> T
    S --> T
    N --> T
Loading

Reviews (22): Last reviewed commit: "mcore integration fixes" | Re-trigger Greptile

Comment thread transformer_engine/pytorch/module/grouped_linear.py
Comment thread tests/pytorch/test_grouped_linear.py Outdated
Comment thread tests/pytorch/test_grouped_mlp.py Outdated

@timmoon10 timmoon10 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The biggest change in this PR is that TE is abandoning any attempt to make single_grouped_weight=True a general feature. Things must be exactly right, or we crash. Given how delicate and experimental this feature has been, I'm not opposed.

The second change is that users must opt-in to access the grouped GEMM kernel. This is also reasonable, since it has alignment requirements for m_splits and it's helpful having a way for users to accept that stricter contract.

We are experiencing many test failures. Given that single_grouped_weight is no longer a general feature, I think it's reasonable we move the corresponding tests to test_grouped_linear.py and test_grouped_mlp.py.

Comment on lines +761 to +764
raise ValueError(
"The native grouped_tensor path requires CUDA m_splits. Pass a CUDA int64 "
"tensor, or select grouped_gemm_backend='legacy'."
)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get that the h2d memcpy is suboptimal, but it's trivially easy to handle. Erroring out seems excessively rigid.

Suggested change
raise ValueError(
"The native grouped_tensor path requires CUDA m_splits. Pass a CUDA int64 "
"tensor, or select grouped_gemm_backend='legacy'."
)
m_splits = m_splits.to(device=device)

We need to handle the d2h case anyways when the user has specified grouped_gemm_backend="grouped_tensor", but it's not supported and we fallback to split-quantize.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But if the alignment is not provided in the first place, converting it to a device tensor also wouldn't work right, I am okay with another alignment check before adding this H2D copy.

Comment thread transformer_engine/pytorch/module/grouped_linear.py Outdated
@zhongbozhu
zhongbozhu force-pushed the improve_device_grouped_linear branch 3 times, most recently from b6a9482 to 25314b8 Compare July 25, 2026 07:08
Comment thread tests/pytorch/test_grouped_linear.py Outdated
@zhongbozhu

Copy link
Copy Markdown
Collaborator Author

/te-ci pytorch

zhongbozhu and others added 6 commits July 27, 2026 20:47
Signed-off-by: Zhongbo Zhu <zhongboz@nvidia.com>
Signed-off-by: Zhongbo Zhu <zhongboz@nvidia.com>
Signed-off-by: Zhongbo Zhu <zhongboz@nvidia.com>

# Conflicts:
#	tests/pytorch/test_grouped_mlp.py
Signed-off-by: Zhongbo Zhu <zhongboz@nvidia.com>
Signed-off-by: Zhongbo Zhu <zhongboz@nvidia.com>
zhongbozhu and others added 11 commits July 27, 2026 20:49
Signed-off-by: Zhongbo Zhu <zhongboz@nvidia.com>
Signed-off-by: Zhongbo Zhu <zhongboz@nvidia.com>
Signed-off-by: Zhongbo Zhu <zhongboz@nvidia.com>
Signed-off-by: Zhongbo Zhu <zhongboz@nvidia.com>
Signed-off-by: Zhongbo Zhu <zhongboz@nvidia.com>
Signed-off-by: Zhongbo Zhu <zhongboz@nvidia.com>
Signed-off-by: zhongboz <zhongboz@nvidia.com>
Signed-off-by: zhongboz <zhongboz@nvidia.com>
Signed-off-by: Zhongbo Zhu <zhongboz@nvidia.com>
@zhongbozhu
zhongbozhu force-pushed the improve_device_grouped_linear branch from 07b2f18 to bb6c1b9 Compare July 28, 2026 03:50
: tensor_base;
size_t tensor_base_for_scales = tensor_base;
size_t tensor_rows_for_scales = rows;
if constexpr (WITH_GEMM_SWIZZLED_SCALES && SHAPE_REP == ShapeRepresentation::SAME_BOTH_DIMS) {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: this is for single weight quantize for mxfp8.

Before this change, the weight quantizer didn't have the first_dims because moe weights are uniform shape for both dimension. This will then trigger a CUDA illegal access because offsets_ptr=nullptr

Signed-off-by: Zhongbo Zhu <zhongboz@nvidia.com>
@zhongbozhu
zhongbozhu force-pushed the improve_device_grouped_linear branch from 71a29ba to 13c60fb Compare July 28, 2026 08:34
Signed-off-by: zhongboz <zhongboz@nvidia.com>
@zhongbozhu
zhongbozhu force-pushed the improve_device_grouped_linear branch from 2734746 to 5fc5db7 Compare July 28, 2026 09:21
Signed-off-by: Zhongbo Zhu <zhongboz@nvidia.com>
Signed-off-by: zhongboz <zhongboz@nvidia.com>
Signed-off-by: zhongboz <zhongboz@nvidia.com>
Comment thread transformer_engine/pytorch/csrc/type_converters.cpp Outdated
Signed-off-by: Zhongbo Zhu <zhongboz@nvidia.com>
@zhongbozhu

Copy link
Copy Markdown
Collaborator Author

/te-ci pytorch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community-contribution PRs from external contributor outside the core maintainers, representing community-driven work.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants